-
Notifications
You must be signed in to change notification settings - Fork 137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for coordinates in Adobe format in XMP tags #653
Conversation
malconsei
commented
Aug 17, 2023
- exif parser: Add support for Adobe coordinates
- exiftool parser: Try to get coordinates in the XMP-exif namespace, too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two cases are doesn't share much in common. It feels easier to understand if you separate them. How about this:
def _extract_lon_lat_numeric():
# the numeric case
pass
def _extract_lon_lat_adobe():
# the adobe case
pass
def extract_lon_lat():
lon_lat = self._extract_lon_lat_numeric()
if lon_lat is not None:
return lon_lat
lon_lat = self. _extract_lon_lat_adobe()
if lon_lat is not None:
return lon_lat
return None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accept to unblock.
mapillary_tools/exif_read.py
Outdated
if lon is None: | ||
return None | ||
|
||
ref = self._extract_alternative_fields(["exif:GPSLongitudeRef"], str) | ||
if ref and ref.upper() == "W": | ||
if ref and ref.upper() == "W" and lon > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to apply the reference for the adobe value again? I thought adobe has NSEW parsed from its own format so it doesn't have to respect this one here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, the Adobe coords already have the right sign, that's why I added the check on lon/lat > 0 - to avoid multiplying them by -1 again. But yeah, it's hacky, I'll make it more explicit :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not separating them? So adoble values don't need to respect the ref tags. See my proposal above.